fix(policy/enforcement): update README for policy 01#416
Conversation
There was a problem hiding this comment.
'We are always happy to welcome new contributors ❤️ To make things easier for everyone, please - make sure to follow our contribution guidelines, - check if you have already signed the ECA, and - relate this pull request to an existing issue or discussion.'
| import org.eclipse.edc.connector.controlplane.catalog.spi.policy.CatalogPolicyContext; | ||
| import static org.eclipse.edc.connector.controlplane.catalog.spi.policy.CatalogPolicyContext.CATALOG_SCOPE; | ||
|
|
||
| ruleBindingRegistry.bind(LOCATION_CONSTRAINT_KEY, CATALOG_SCOPE); | ||
| policyEngine.registerFunction(CatalogPolicyContext.class, Permission.class, LOCATION_CONSTRAINT_KEY, new LocationConstraintFunction(monitor)); |
There was a problem hiding this comment.
I'm not sure about adding these imports, nowadays IDEs are pretty good in doing that automatically.
At least please add some points (...) in the middle to show that these lines needs to go in different part of the class
There was a problem hiding this comment.
In my case (using IntelliJ IDEA 2025.1.2 Community Edition), the import wasn’t added automatically, so I included it manually.
I also already added some points (...) to show that these lines belong in different parts of the class.
That said, it's completely up to you whether to keep this part :)
There was a problem hiding this comment.
In my case (using IntelliJ IDEA 2025.1.2 Community Edition), the import wasn’t added automatically, so I included it manually.
maybe not 100% automatically, but with a simple Alt+Enter it should suggest the correct options, to me that's pretty automatic
I also already added some points (...) to show that these lines belong in different parts of the class.
That said, it's completely up to you whether to keep this part :)
What do you mean?
There was a problem hiding this comment.
Okay, i already deleted the unnecessary imports.
|
@ortegi you need to sign the ECA to being able to contribute (see the related broken check), also to rename the PR to follow the project convention. |
Co-authored-by: andrea bertagnolli <andrea.bertagnolli@gmail.com>
Co-authored-by: andrea bertagnolli <andrea.bertagnolli@gmail.com>
What this PR changes/adds
This PR updates the policy-01-policy-enforcement/README.md file to fix and clarify the instructions for running and testing the provided code.
Context
While going through the README to test the code, I noticed that some of the final instructions (Bind the constraint to the cataloging scope) didn’t work as expected. After a bit of trial and error, I found that a few parts were either outdated or incorrect.
This is what i found:
The CATALOGING_SCOPE doesn’t seem to exist. The actual constant is CATALOG_SCOPE, and the correct context class to use with policyEngine.registerFunction is CatalogPolicyContext.class.
The import statement for the correct constant (CATALOG_SCOPE) was missing.
A code change in LocationConstraintFunction.java is also needed. This wasn't mentioned in the README, but in order for everything to work, the LocationConstraintFunction class needs to be updated to use CatalogPolicyContext instead of ContractNegotiationPolicyContext.
Who will sponsor this feature?
ndr-brt
Linked Issue(s)
Closes #245
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.